-
Notifications
You must be signed in to change notification settings - Fork 433
Sync node help with selection and add watcher tests #7105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughRefactors the selection-state composable into an internal implementation with a public wrapper cached per-Pinia instance, adds a whenever watcher to synchronize the help panel when selection changes while help is open, and adds tests covering the help-sync behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as User / UI
participant Sel as useSelectionState (composable)
participant Pin as Pinia (currentHelpNode)
participant Help as HelpPanel (openHelp)
UI->>Sel: selectNode(nodePath)
Sel->>Sel: update selectedNodes / selection state
Sel->>Pin: read currentHelpNode (optional)
alt help panel is open AND nodePath != currentHelpNode?.nodePath
Sel->>Help: openHelp(nodeDef) %% watcher invokes openHelp for new node
Help-->>Pin: set currentHelpNode(nodePath)
else help closed OR same node
Sel-->>Help: (no action)
end
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎭 Playwright Test Results⏰ Completed at: 12/06/2025, 05:25:43 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/06/2025, 05:16:41 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.2 MB (baseline 3.2 MB) • ⚪ 0 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 975 kB (baseline 974 kB) • 🔴 +747 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 298 kB (baseline 298 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 177 kB (baseline 177 kB) • ⚪ 0 BReusable component library chunks
Status: 6 added / 6 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 3 added / 3 removed Utilities & Hooks — 2.94 kB (baseline 2.94 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 8.56 MB (baseline 8.56 MB) • 🔴 +155 BExternal libraries and shared vendor chunks
Status: 1 added / 1 removed Other — 3.81 MB (baseline 3.81 MB) • ⚪ 0 BBundles that do not match a named category
Status: 17 added / 17 removed |
|
It seems the test is broken? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds automatic synchronization of the node help panel with the currently selected node and includes comprehensive unit tests. The composable is refactored to use Vue's shared composable pattern for better instance management.
- Implements a watcher using VueUse's
wheneverto automatically sync the help panel when selection changes - Adds unit tests covering help auto-switching behavior and guard conditions
- Refactors
useSelectionStateto usecreateSharedComposablefor proper instance sharing
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/composables/graph/useSelectionState.ts | Adds whenever watcher for help panel sync, wraps composable with createSharedComposable, and improves type safety by removing any annotations |
| tests-ui/tests/composables/graph/useSelectionState.test.ts | Adds test infrastructure with component mounting helpers, updates existing tests to use mounted components, adds new test suite for help sync behavior, and includes proper cleanup in afterEach |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests-ui/tests/composables/graph/useSelectionState.test.ts (1)
186-204: Critical: MockisHelpOpenmust be reactive for watcher tests to work correctly.The mock
nodeHelpStore.isHelpOpenis a plain boolean property, but in the real store it's a computed ref. The watcher inuseSelectionStatewatches() => (nodeHelpStore.isHelpOpen ? nodeDef.value : null), which depends onisHelpOpenbeing reactive. Directly assigning to the mock property (lines 356, 362) won't trigger the watcher because Vue's reactivity system won't track changes to plain properties.To fix this, make the mock reactive using a ref with a getter:
+ const mockIsHelpOpen = ref(false) + const mockCurrentHelpNode = ref(null) const mockOpenHelp = vi.fn() const mockCloseHelp = vi.fn() const mockNodeHelpStore = { - isHelpOpen: false, - currentHelpNode: null, + get isHelpOpen() { + return mockIsHelpOpen.value + }, + set isHelpOpen(value: boolean) { + mockIsHelpOpen.value = value + }, + get currentHelpNode() { + return mockCurrentHelpNode.value + }, + set currentHelpNode(value: any) { + mockCurrentHelpNode.value = value + }, openHelp: mockOpenHelp, closeHelp: mockCloseHelp, // Add minimal required properties for the storeThen update the test to use the refs:
test('opens help for newly selected node when help is open', async () => { const nodeHelpStore = useNodeHelpStore() as any - nodeHelpStore.isHelpOpen = true - nodeHelpStore.currentHelpNode = { nodePath: 'NodeA' } + mockIsHelpOpen.value = true + mockCurrentHelpNode.value = { nodePath: 'NodeA' }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/composables/graph/useSelectionState.ts(5 hunks)tests-ui/tests/composables/graph/useSelectionState.test.ts(7 hunks)
🧰 Additional context used
📓 Path-based instructions (16)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
src/composables/graph/useSelectionState.tstests-ui/tests/composables/graph/useSelectionState.test.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/composables/graph/useSelectionState.tstests-ui/tests/composables/graph/useSelectionState.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/composables/graph/useSelectionState.tstests-ui/tests/composables/graph/useSelectionState.test.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
src/composables/graph/useSelectionState.tstests-ui/tests/composables/graph/useSelectionState.test.ts
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/composables/graph/useSelectionState.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/composables/graph/useSelectionState.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/composables/graph/useSelectionState.tstests-ui/tests/composables/graph/useSelectionState.test.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
src/composables/graph/useSelectionState.tstests-ui/tests/composables/graph/useSelectionState.test.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
src/composables/graph/useSelectionState.tstests-ui/tests/composables/graph/useSelectionState.test.ts
src/**/{services,composables}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/{services,composables}/**/*.{ts,tsx}: Useapi.apiURL()for backend endpoints instead of constructing URLs directly
Useapi.fileURL()for static file access instead of constructing URLs directly
Files:
src/composables/graph/useSelectionState.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/composables/graph/useSelectionState.ts
src/**/{composables,components}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Clean up subscriptions in state management to prevent memory leaks
Files:
src/composables/graph/useSelectionState.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/composables/graph/useSelectionState.ts
src/**/{components,composables}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Use vue-i18n for ALL user-facing strings by adding them to
src/locales/en/main.json
Files:
src/composables/graph/useSelectionState.ts
tests-ui/**/*.test.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (tests-ui/CLAUDE.md)
tests-ui/**/*.test.{js,ts,jsx,tsx}: Write tests for new features
Follow existing test patterns in the codebase
Use existing test utilities rather than writing custom utilities
Mock external dependencies in tests
Always prefer vitest mock functions over writing verbose manual mocks
Files:
tests-ui/tests/composables/graph/useSelectionState.test.ts
**/*.{test,spec}.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Unit and component tests should be located in
tests-ui/or co-located with components assrc/components/**/*.{test,spec}.ts; E2E tests should be inbrowser_tests/
Files:
tests-ui/tests/composables/graph/useSelectionState.test.ts
🧠 Learnings (27)
📓 Common learnings
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features
📚 Learning: 2025-11-24T19:46:52.279Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T19:46:52.279Z
Learning: Applies to **/*.{vue,ts,tsx} : Leverage VueUse functions for performance-enhancing utilities
Applied to files:
src/composables/graph/useSelectionState.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Extract complex conditionals to computed properties
Applied to files:
src/composables/graph/useSelectionState.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Use ref/reactive for state management in Vue 3 Composition API
Applied to files:
src/composables/graph/useSelectionState.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Implement computed() for derived state in Vue 3 Composition API
Applied to files:
src/composables/graph/useSelectionState.ts
📚 Learning: 2025-11-24T19:46:52.279Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T19:46:52.279Z
Learning: Applies to **/*.vue : Utilize ref and reactive for reactive state in Vue 3
Applied to files:
src/composables/graph/useSelectionState.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Utilize ref and reactive for reactive state
Applied to files:
src/composables/graph/useSelectionState.tstests-ui/tests/composables/graph/useSelectionState.test.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Use setup() function in Vue 3 Composition API
Applied to files:
src/composables/graph/useSelectionState.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Use lifecycle hooks: onMounted, onUpdated in Vue 3 Composition API
Applied to files:
src/composables/graph/useSelectionState.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Implement computed properties with computed()
Applied to files:
src/composables/graph/useSelectionState.ts
📚 Learning: 2025-11-24T19:46:52.279Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T19:46:52.279Z
Learning: Applies to **/*.vue : Implement computed properties with computed() function
Applied to files:
src/composables/graph/useSelectionState.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : Use provided test helpers `createTestSubgraph` and `createTestSubgraphNode` from `./fixtures/subgraphHelpers` for consistent subgraph test setup
Applied to files:
tests-ui/tests/composables/graph/useSelectionState.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features
Applied to files:
tests-ui/tests/composables/graph/useSelectionState.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Use existing test utilities rather than writing custom utilities
Applied to files:
tests-ui/tests/composables/graph/useSelectionState.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Follow existing test patterns in the codebase
Applied to files:
tests-ui/tests/composables/graph/useSelectionState.test.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : When writing tests for subgraph-related code, always import from the barrel export at `@/lib/litegraph/src/litegraph` to avoid circular dependency issues
Applied to files:
tests-ui/tests/composables/graph/useSelectionState.test.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Use Suspense for async components
Applied to files:
tests-ui/tests/composables/graph/useSelectionState.test.ts
📚 Learning: 2025-11-24T19:46:52.279Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T19:46:52.279Z
Learning: Applies to **/*.vue : Use Suspense for async components in Vue 3
Applied to files:
tests-ui/tests/composables/graph/useSelectionState.test.ts
📚 Learning: 2025-11-24T19:48:23.088Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T19:48:23.088Z
Learning: Use Vitest (with happy-dom) for unit and component tests, and Playwright for E2E tests
Applied to files:
tests-ui/tests/composables/graph/useSelectionState.test.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Use Teleport/Suspense when needed for component rendering
Applied to files:
tests-ui/tests/composables/graph/useSelectionState.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Check tests-ui/README.md for test guidelines
Applied to files:
tests-ui/tests/composables/graph/useSelectionState.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Always prefer vitest mock functions over writing verbose manual mocks
Applied to files:
tests-ui/tests/composables/graph/useSelectionState.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Mock external dependencies in tests
Applied to files:
tests-ui/tests/composables/graph/useSelectionState.test.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Use `vitest` for unit testing in this project
Applied to files:
tests-ui/tests/composables/graph/useSelectionState.test.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Prefer the use of `test.extend` over loose variables; import `test as baseTest` from `vitest`
Applied to files:
tests-ui/tests/composables/graph/useSelectionState.test.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Use `test` instead of `it` for defining test cases in vitest
Applied to files:
tests-ui/tests/composables/graph/useSelectionState.test.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Mocks should be cleanly written and easy to understand, with reusable mocks where possible
Applied to files:
tests-ui/tests/composables/graph/useSelectionState.test.ts
🧬 Code graph analysis (1)
tests-ui/tests/composables/graph/useSelectionState.test.ts (2)
src/composables/graph/useSelectionState.ts (1)
useSelectionState(156-158)browser_tests/fixtures/utils/litegraphUtils.ts (2)
isBypassed(354-356)isPinned(348-350)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Agent
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: test
- GitHub Check: collect
🔇 Additional comments (5)
tests-ui/tests/composables/graph/useSelectionState.test.ts (2)
84-126: LGTM: Well-structured mount helpers.The mount helpers follow Vue Test Utils best practices and properly integrate with the component lifecycle. The
mountHelpSyncHarnessprovides a clean way to test the watcher behavior through user interactions.
228-230: LGTM: Proper cleanup prevents memory leaks.The
afterEachhook correctly unmounts all test components, preventing memory leaks between tests.src/composables/graph/useSelectionState.ts (3)
1-1: LGTM: Proper use of VueUse utilities.Using
createSharedComposableensures all callers get the same instance, which is essential for the help sync watcher to work correctly. This prevents multiple watcher instances from independently trying to manage the help panel.As per coding guidelines: Leveraging VueUse functions for performance-enhancing utilities.
Also applies to: 156-158
98-107: LGTM: Well-implemented help synchronization.The
wheneverwatcher correctly:
- Watches a conditional expression that returns
nodeDefonly when help is open- Includes a guard to prevent redundant
openHelpcalls when the same node is already displayed- Leverages VueUse's type narrowing to exclude null from the callback parameter
The logic ensures help content stays synchronized with the selected node when the panel is open.
120-124: LGTM: Safer type handling with optional chaining.The optional chaining on
currentHelpNode?.nodePathproperly handles the case wherecurrentHelpNodeisnull, which aligns with its type definitionRef<ComfyNodeDefImpl | null>.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
browser_tests/tests/nodeHelp.spec.ts(0 hunks)src/composables/graph/useSelectionState.ts(5 hunks)
💤 Files with no reviewable changes (1)
- browser_tests/tests/nodeHelp.spec.ts
🧰 Additional context used
📓 Path-based instructions (10)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/composables/graph/useSelectionState.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/composables/graph/useSelectionState.ts
src/**/{services,composables}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/{services,composables}/**/*.{ts,tsx}: Useapi.apiURL()for backend endpoints instead of constructing URLs directly
Useapi.fileURL()for static file access instead of constructing URLs directly
Files:
src/composables/graph/useSelectionState.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/composables/graph/useSelectionState.ts
src/**/{composables,components}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Clean up subscriptions in state management to prevent memory leaks
Files:
src/composables/graph/useSelectionState.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/composables/graph/useSelectionState.ts
src/**/{components,composables}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Use vue-i18n for ALL user-facing strings by adding them to
src/locales/en/main.json
Files:
src/composables/graph/useSelectionState.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript exclusively; no new JavaScript code
Files:
src/composables/graph/useSelectionState.ts
**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,vue}: Use camelCase for variable and function names
Indent with 2 spaces (see.prettierrc)
Use single quotes for strings (see.prettierrc)
No trailing semicolons (see.prettierrc)
Maximum line width of 80 characters (see.prettierrc)
Sort and group imports by plugin (runpnpm formatbefore committing)
Never useanytype; use proper TypeScript types instead
Never useas anytype assertions; fix the underlying type issue instead
Avoid code comments unless absolutely necessary; write expressive, self-documenting code instead
When writing new code, ask if there is a simpler way to introduce the same functionality; if yes, choose the simpler approach
Use refactoring to make complex code simpler
Use es-toolkit for utility functions
Use Vite for fast development and building
Implement proper error handling
Write tests for all changes, especially bug fixes to catch future regressions
Files:
src/composables/graph/useSelectionState.ts
**/composables/**/use*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name composables with
useXyz.tspattern
Files:
src/composables/graph/useSelectionState.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/{composables,components}/**/*.{ts,tsx,vue} : Clean up subscriptions in state management to prevent memory leaks
Applied to files:
src/composables/graph/useSelectionState.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.vue : Use `computed` instead of a `ref` and `watch` if possible
Applied to files:
src/composables/graph/useSelectionState.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Extract complex conditionals to computed properties
Applied to files:
src/composables/graph/useSelectionState.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Utilize ref and reactive for reactive state
Applied to files:
src/composables/graph/useSelectionState.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Use ref/reactive for state management in Vue 3 Composition API
Applied to files:
src/composables/graph/useSelectionState.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: collect
🔇 Additional comments (3)
src/composables/graph/useSelectionState.ts (3)
1-2: LGTM!Good use of
wheneverfrom VueUse (preferred over manualref+watchper coding guidelines) andgetActivePiniafor the per-Pinia instance caching pattern.
98-107: LGTM —wheneverensuresdefis truthy.The
whenevercallback only fires when the source returns a truthy value. Since the getter returnsnullwhen help is closed or when there's no single-node selection,defwill always be the valid node definition when the callback executes.The previous Copilot suggestion for a null check is unnecessary here due to
whenever's semantics.
120-124: LGTM!Good use of optional chaining for null-safety, consistent with the pattern in the
whenevercallback.
Summary
Testing
┆Issue is synchronized with this Notion page by Unito